Skip to content
This repository has been archived by the owner on Jun 13, 2019. It is now read-only.

Implement local testing (#13) #16

Merged
merged 1 commit into from
Oct 27, 2017

Conversation

ChristopherMacGown
Copy link
Contributor

No description provided.

@iliana
Copy link
Owner

iliana commented Oct 25, 2017

I feel like I should just be able to run make test and Make should compile the examples before running the test commands. Is that an improvement you'd like to write?

@ChristopherMacGown
Copy link
Contributor Author

Yep, I can do that. It will probably take a Makefile in each example directory to make the recursive build work properly. Is that ok?

@iliana
Copy link
Owner

iliana commented Oct 25, 2017

Hm. Would cd examples/echo; cargo build --release work too?

If all that's happening is execing cargo in a directory I'm not particularly thrilled about there being a Makefile there too (especially since the example dirs are designed as much as possible to be cargo culted to new projects).

@ChristopherMacGown ChristopherMacGown force-pushed the support-local-testing branch 3 times, most recently from ff5af0b to 148b3c3 Compare October 25, 2017 21:46
@ChristopherMacGown
Copy link
Contributor Author

I've moved the Makefile out of the root of the repo into test/ and split it up into two sub-Makefiles Makefile.local and Makefile.docker. The .local version allows you to build and test the examples locally, and the .docker version builds them via the docker builder and tests them against amazonlinux:latest.

I also added a commit that removes the PYENV references from builder/build.sh because that fails as well due to the unbound variable.

@iliana
Copy link
Owner

iliana commented Oct 26, 2017

I also added a commit that removes the PYENV references from builder/build.sh because that fails as well due to the unbound variable.

oh whoops. :(

I'll hopefully be able to test and merge this tonight.

@iliana
Copy link
Owner

iliana commented Oct 26, 2017

I'm actually going to go ahead and merge #18 which will mark this PR as conflicting with the master branch, but I can fix that myself later tonight if all goes smoothly.

@ChristopherMacGown
Copy link
Contributor Author

No worries, I dropped the conflicting commit.

@iliana iliana merged commit cee7167 into iliana:master Oct 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants